feat(CC-batch-6): added batch 6 #11868

mattnolting · 2025-06-12T11:53:46Z

Relates to: #11624

Included components:

Component tracker

Figma preview

Resources:

Getting started

patternfly-build · 2025-06-12T11:57:59Z

Preview: https://patternfly-react-pr-11868.surge.sh

A11y report: https://patternfly-react-pr-11868-a11y.surge.sh

Preview: https://pf-react-pr-11868.surge.sh

A11y report: https://pf-react-pr-11868-a11y.surge.sh

packages/code-connect/components/JumpLinks/JumplinkVertical.figma.tsx

packages/code-connect/components/JumpLinks/JumplinkHorizontal.figma.tsx

packages/code-connect/components/Label/LabelNonStatus.figma.tsx

packages/code-connect/components/Label/LabelStatus.figma.tsx

packages/code-connect/components/LoginPage/DesktopLoginPage.figma.tsx

packages/code-connect/components/Masthead/Masthead.figma.tsx

nicolethoen · 2025-06-12T14:38:44Z

packages/code-connect/components/Masthead/MastheadMenuItemToggles.figma.tsx

+    props: {
+      children: figma.children('*')
+    },
+    example: (props) => <MastheadToggle>{props.children}</MastheadToggle>


should we tell them that this is intended to be a button with an onClick handler for toggling open and closed the sidebar nav?

I think it would be a good comment to have.

nicolethoen · 2025-06-12T14:42:46Z

packages/code-connect/components/Modal/BasicModal.figma.tsx

+    },
+    example: (props) => (
+      // Documentation for Modal can be found at https://www.patternfly.org/components/modal
+      <Modal variant={props.size}>{props.children}</Modal>


This will have the same issues as the AboutModal where the Modal will not appear unless it has isOpen={true} but really they probably want a button which will allow them to open/close the modal...

This also invalid...
It needs a header, body, and footer. It'll also need a close button of some kind

This applies for the AlertModal figma cc as well, it also needs the header/body/footer. children probably maps to the ModalBody content I would expect, and the rest could be stubbed out.

Modal would also benefit from having aria-labelledby and aria-describedby stubbed out with ids matching the respective ModalHeader and ModalBody.

nicolethoen · 2025-06-12T14:45:29Z

packages/code-connect/components/Modal/AlertModal.figma.tsx

+    },
+    example: (props) => (
+      // Documentation for Modal can be found at https://www.patternfly.org/components/modal
+      <Modal variant={props.size}>{props.children}</Modal>


we don't have alert modals?

we have a title icon which lets them specify a status to add the correct icon to the header?
https://www.patternfly.org/components/modal#title-icon

packages/code-connect/components/JumpLinks/JumplinkHorizontal.figma.tsx

packages/code-connect/components/Label/LabelNonStatus.figma.tsx

packages/code-connect/components/Masthead/Masthead.figma.tsx

kmcfaul · 2025-06-12T18:35:54Z

packages/code-connect/components/Masthead/MastheadMenuItemToggles.figma.tsx

+    props: {
+      children: figma.children('*')
+    },
+    example: (props) => <MastheadToggle>{props.children}</MastheadToggle>


I think it would be a good comment to have.

kmcfaul · 2025-06-12T18:39:06Z

packages/code-connect/components/Modal/BasicModal.figma.tsx

+    },
+    example: (props) => (
+      // Documentation for Modal can be found at https://www.patternfly.org/components/modal
+      <Modal variant={props.size}>{props.children}</Modal>


This applies for the AlertModal figma cc as well, it also needs the header/body/footer. children probably maps to the ModalBody content I would expect, and the rest could be stubbed out.

Modal would also benefit from having aria-labelledby and aria-describedby stubbed out with ids matching the respective ModalHeader and ModalBody.

nicolethoen

Only one issue I see in this last pass through

nicolethoen · 2025-09-05T13:32:35Z

packages/code-connect/components/Label/LabelGroups.figma.tsx

+        addLabelControl={props.addLabelControl}
+        categoryName="Group label"
+        isClosable
+        isEditable


IsClosable & isEditable are both features that i'd imagine are not present in most label groups and I would be suprised if they are enabled on all Figma designs. We might want to talk through the types of LabelGroups that designers can choose between and make sure each feature is implemented intentionally.

Great point, I opened #1451 to track discussion topics moving forward.

Also, I left isClosable and removed isEditable for the current integration. This approach aligns with figma's component presentation.

kmcfaul · 2025-09-09T17:03:15Z

packages/code-connect/components/JumpLinks/JumplinksHorizontal.figma.tsx

+
+      children: figma.children('*')
+    },
+    example: (props) => <JumpLinks label={props.label}>{props.children}</JumpLinks>


Probably want to set aria-label here since label isn't always defined (by default it gets set to the label).

kmcfaul · 2025-09-09T17:03:33Z

packages/code-connect/components/JumpLinks/JumplinksVertical.figma.tsx

+      children: figma.children('*')
+    },
+    example: (props) => (
+      <JumpLinks isVertical label={props.label}>


Same aria-label note as above.

kmcfaul · 2025-09-09T17:04:22Z

packages/code-connect/components/Label/LabelGroups.figma.tsx

+  {
+    props: {
+      // string
+      labelGroupName: figma.string('Label group name'),


This is unused.

kmcfaul · 2025-09-09T17:06:43Z

packages/code-connect/components/Label/LabelNonStatus.figma.tsx

+    example: (props) => (
+      <Label isCompact={props.isCompact} isEditable={props.isEditable} color={props.color} variant={props.variant}>
+        {props.children}
+        {props.labelText}


labelText and children seem to be redundant here, unless Figma has multiple content items?

@kmcfaul updated! Icon is now returned by via status prop.

kmcfaul · 2025-09-09T17:09:46Z

packages/code-connect/components/LoginPage/DesktopLoginPage.figma.tsx

+        socialMediaLoginContent={socialMediaLoginContent}
+        socialMediaLoginAriaLabel="Log in with social media"
+        signUpForAccountMessage={signUpForAccountMessage}
+        forgotCredentials={forgotCredentialsprops.isInvalid}


There are a lot of undefined props here, but I think this may be okay enough for a LoginPage stub. wdyt @nicolethoen?

Discussed w/@kmcfaul, we think its configuration is acceptable.

kmcfaul · 2025-09-09T17:11:19Z

packages/code-connect/components/Masthead/Masthead.figma.tsx

+    },
+    example: (props) => {
+      /* eslint-disable */
+      const [activeItem, setActiveItem] = useState(0);


We haven't been putting stubs in for state in other components so I'm unsure if we need it, but I think it's probably fine since activeItem is referenced in Nav block.

kmcfaul · 2025-09-09T17:13:59Z

packages/code-connect/components/Modal/AlertModal.figma.tsx

+            aria-labelledby="<modal-id>"
+            aria-describedby="<modal-body>"
+            isOpen={true}
+            onClick={() => {


Should be onClose.

kmcfaul · 2025-09-09T17:14:28Z

packages/code-connect/components/Modal/BasicModal.figma.tsx

+            aria-labelledby="<modal-id>"
+            aria-describedby="<modal-body>"
+            isOpen={true}
+            onClose={() => {}} // handles the close button


Should call setIsOpen(!isOpen) if we're going to stub out the state.

mattnolting · 2025-09-15T14:31:10Z

@kmcfaul I've addressed all of your comments/questions. It's ready for re-review. Side note: there are comments (top of file) noting the need for prop assignment for designers.

mattnolting · 2025-09-23T19:06:04Z

packages/code-connect/components/Label/LabelInGroup.figma.tsx

+        false: undefined
+      })
+    },
+    example: (props) => <Label onClose={props.isCloseable}>{props.labelText}</Label>


add hasCounter
add count
add hasCloseButton

mattnolting · 2025-09-23T19:08:41Z

packages/code-connect/components/Label/LabelOverflow.figma.tsx

+    // string
+    text: figma.string('Text'),
+


not using text prop

mattnolting · 2025-09-23T19:10:02Z

packages/code-connect/components/LoginPage/DesktopLoginPage.figma.tsx

+        socialMediaLoginContent={socialMediaLoginContent}
+        socialMediaLoginAriaLabel="Log in with social media"
+        signUpForAccountMessage={signUpForAccountMessage}
+        forgotCredentials={forgotCredentialsprops.isInvalid}


Discussed w/@kmcfaul, we think its configuration is acceptable.

mattnolting marked this pull request as ready for review June 12, 2025 11:57

mattnolting requested review from evwilkin and kmcfaul June 12, 2025 11:57

nicolethoen self-requested a review June 12, 2025 14:13

nicolethoen requested changes Jun 12, 2025

View reviewed changes

kmcfaul reviewed Jun 12, 2025

View reviewed changes

mattnolting marked this pull request as draft June 17, 2025 14:54

mattnolting marked this pull request as ready for review June 24, 2025 15:22

mattnolting force-pushed the feat-CC-batch-6 branch from f5c92f6 to a855dc5 Compare June 30, 2025 20:27

mattnolting force-pushed the feat-CC-batch-6 branch 3 times, most recently from 1a3bec5 to 92b1f2d Compare July 31, 2025 19:31

mattnolting force-pushed the feat-CC-batch-6 branch 2 times, most recently from 00d0f3f to 6ba426e Compare September 4, 2025 14:46

nicolethoen reviewed Sep 5, 2025

View reviewed changes

mattnolting mentioned this pull request Sep 5, 2025

Figma Code Connect Optimization patternfly/patternfly-design#1451

Closed

1 task

nicolethoen approved these changes Sep 5, 2025

View reviewed changes

kmcfaul reviewed Sep 9, 2025

View reviewed changes

mattnolting force-pushed the feat-CC-batch-6 branch from 4792e60 to 3fe910c Compare September 15, 2025 13:50

mattnolting commented Sep 23, 2025

View reviewed changes

mattnolting added 8 commits September 23, 2025 16:07

feat(CC-batch-6): added batch 6 components

de8c717

update

feba35a

feat(CC-batch-6): updated required props

a8b7d30

feat(CC-batch-6): verification complete

8cc1216

feat(CC-batch-6): review-athon round 1 followup

a98dff6

feat(CC-batch-6): group review 6 updates

8e8d698

feat(CC-batch-6): removed isEditable from label group

b587cc7

feat(CC-batch-6): @kmcfaul review 7 updates

9b5a068

mattnolting added 2 commits September 23, 2025 16:07

feat(CC-batch-6): @kmcfaul review 7 updates

6cdffaa

feat(CC-batch-6): review 7 updated modal onClose()

4e8046b

mattnolting force-pushed the feat-CC-batch-6 branch from 8cfe9ca to fb7ed13 Compare September 23, 2025 20:07

feat(CC-batch-6): group review 9/23 updates

af3eefe

mattnolting force-pushed the feat-CC-batch-6 branch from fb7ed13 to af3eefe Compare September 23, 2025 20:22

nicolethoen merged commit d316364 into patternfly:main Oct 24, 2025
13 checks passed

feat(CC-batch-6): added batch 6 #11868

feat(CC-batch-6): added batch 6 #11868

Uh oh!

Conversation

mattnolting commented Jun 12, 2025

Uh oh!

patternfly-build commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolethoen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattnolting commented Sep 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

patternfly-build commented Jun 12, 2025 •

edited

Loading